Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Web Share (#286) #341

Merged
merged 19 commits into from
Jan 4, 2022
Merged

feat: Web Share (#286) #341

merged 19 commits into from
Jan 4, 2022

Conversation

valentinoli
Copy link
Contributor

@valentinoli valentinoli commented Dec 17, 2021

Implemented a share button that uses the Web Share API with a polyfill.

See issue

I've implemented a BaseShareButton component. It can be used for file sharing too when the device/browser supports it. I added the share button to UnitFilterableTable to share the exported CSV of a data table, and to UnitVegaViz to share the viz as an image. The share button in the top right corner of an experience is just for sharing meta-info about the experience page - url, title, text. I append the hashtags to the text. These properties are also included when a file is shared but can be overwritten.

The Windows native share dialog is quite limiting. I know that the Twitter Windows app supports it (although CSV sharing does not seem to work). For Windows we might want to consider turning it off and using the polyfill instead but this we can discuss.

Polyfill
Note that the polyfill package does not support file sharing and has some bugs (I have submitted PRs for some) but it is pretty nice!
My PRs:
https://github.com/on2-dev/share-api-polyfill/pulls/valentinoli

@valentinoli valentinoli changed the title Web Share (#286) feat: Web Share (#286) Dec 17, 2021
@valentinoli
Copy link
Contributor Author

might want to use this https://github.com/1904labs/dom-to-image-more

for the ChartView component to produce a single image

instead of extracting all the d3 vizualization SVG elements in the ChartView and creating an image for each one

Copy link
Contributor

@ffsinger ffsinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have updated all the dependencies in package-lock.json (e.g. webpack has been updated to version 5).

I made the same mistake a few days ago in #328, and fixed in e6adc80

Otherwise, code looks good

@valentinoli
Copy link
Contributor Author

You have updated all the dependencies in package-lock.json (e.g. webpack has been updated to version 5).

I made the same mistake a few days ago in #328, and fixed in e6adc80

Otherwise, code looks good

Oh no! I had some issues with that... thanks for noticing.

@valentinoli valentinoli requested a review from ffsinger December 17, 2021 21:29
@ffsinger
Copy link
Contributor

ffsinger commented Dec 20, 2021

The button at the top right works fine for me, but the other "Share" buttons in the viz/tables are not visible on my machine (Linux, Firefox or Chromium), even though they are listed in the component hierarchy of vue-dev-utils. Do you have an idea of what could be the problem?

Edit: Ok it seems that condition is undefined. If I set it manually to true, the button appears, and when I click on it it says "Your system does not support sharing files". There's probably no way around it but in this case it doesn't really make sense to show the "Export" button

Edit 2: Maybe this will be fixed with your PRs in the polyfill repo ?

@valentinoli
Copy link
Contributor Author

valentinoli commented Dec 20, 2021

You are right Florian that if there is no share button the export button is not needed but I found it clumsy and even more inconsistent to remove it... Also, the code becomes cleaner and the old workaround for the export+download in 1 click of the download button is not needed...

Downside is that the user needs to first export and then download...

Edit:
FYI, I tried to integrate the exporting into the click of the share button but I got an error in Chrome saying that the call for sharing needs to happen as a direct consequence of a user gesture.

@ffsinger
Copy link
Contributor

I agree that for the table it's fine to have the Export and Download as separate buttons if the share option is not possible. But for the visualizations it's confusing to have this export button that does nothing, e.g. for the twitter advertising overview it's like this:

Screenshot_20211220_112928

@valentinoli
Copy link
Contributor Author

Ok, that is not good

I have to take a better look

+ Add ChartViewVRowWebShare component that is a VRow with a VCol at the end for image exporting, downloading and sharing of the VRow node
+ Create a mixin for the export image functionality and use it both for chart views and vega visualizations
+ Use dom-to-image-more library to export a DOM node to an image
@valentinoli
Copy link
Contributor Author

valentinoli commented Dec 25, 2021

@ffsinger
Now the Export button should never appear on its own. I forgot to add a Download button beside it.
By the way, I created a component ChartViewVRowWebShare that is basically a VRow with the export/download/sharing functionality. Clicking "Export" is exporting the entire content of the VRow as an image.

Now the SVGs are not exported separately but the entire VRow DOM as a single image using dom-to-image-more as suggested by @andreaskundig. I decided to use the same implementation for the vega visualizations so I encapsulated the functionality in a mixin. Note that there is a bug I reported when the node is an iframe, so applying this to the Kepler iframe needs to wait...

Finally, the background of the images is white now and not transparent.

Copy link
Contributor

@ffsinger ffsinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ChartViewSunburst should also use ChartViewVRowWebShare

Otherwise it looks good now

@valentinoli
Copy link
Contributor Author

@andreaskundig your review is pending

@andreaskundig
Copy link
Contributor

@valentinoli I took this week off, but one approval is enough for a pull request, you can merge it.

@valentinoli valentinoli merged commit fc55b5c into master Jan 4, 2022
@ffsinger ffsinger deleted the feat/web-share-api branch January 5, 2022 12:55
@andreaskundig andreaskundig restored the feat/web-share-api branch January 24, 2022 14:47
@valentinoli valentinoli deleted the feat/web-share-api branch April 12, 2022 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants